Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get package name from pyproject.toml, allow arbitrary files in packages #101

Merged
merged 5 commits into from
Dec 3, 2023

Conversation

jepler
Copy link
Member

@jepler jepler commented Nov 29, 2023

This is more dependable, and when we know the package name we can glob inside it to get all files such as bin or ttf files.

This will allow e.g., 5x8.bin & ov5640_autofocus.bin within
bundles.

the behavior of bundlefly and circup when encountering .bin files needs to be checked.

Tested by building modified pycamera bundle and the autofocus.bin file appears in the generated zip files:

pycamera-py-ec67bde/lib/adafruit_pycamera/ov5640_autofocus.bin 4077 4096
pycamera-8.x-mpy-ec67bde/lib/adafruit_pycamera/ov5640_autofocus.bin 4077 4096
pycamera-9.x-mpy-ec67bde/lib/adafruit_pycamera/ov5640_autofocus.bin 4077 4096

There's at least one library in the bundle that has incorrect metadata and that leads to an error:
adafruit/Adafruit_CircuitPython_Colorsys#29

@jepler jepler requested review from FoamyGuy and a team November 29, 2023 23:41
This is more dependable, and when we know the package name we can
glob inside it to get all files such as bin or ttf files.

This will allow e.g., 5x8.bin & ov5640_autofocus.bin within
bundles.

the behavior of bundlefly and circup when encountering .bin files
needs to be checked.

Tested by building modified pycamera bundle and the autofocus.bin file
appears in the generated zip files:
```
pycamera-py-ec67bde/lib/adafruit_pycamera/ov5640_autofocus.bin 4077 4096
pycamera-8.x-mpy-ec67bde/lib/adafruit_pycamera/ov5640_autofocus.bin 4077 4096
pycamera-9.x-mpy-ec67bde/lib/adafruit_pycamera/ov5640_autofocus.bin 4077 4096
```

There's at least one library in the bundle that has incorrect metadata
and that leads to an error:
    adafruit/Adafruit_CircuitPython_Colorsys#29
@jepler jepler force-pushed the include-binary-files-in-package branch from 6de3690 to 033bf0a Compare November 30, 2023 15:08
@jepler jepler changed the title Allow any ".bin" file to be shipped in a circuitpython package Get package name from settings.toml, allow arbitrary files in packages Nov 30, 2023
@jepler
Copy link
Member Author

jepler commented Nov 30, 2023

circup appears to use shutil.copytree, so it will copy files regardless of type/extension.

@jepler jepler force-pushed the include-binary-files-in-package branch from 033bf0a to b2116cb Compare November 30, 2023 15:11
@jepler
Copy link
Member Author

jepler commented Nov 30, 2023

This'll require some metadata problems across multiple libs to be corrected.

jepler added a commit to jepler/CircuitPython_Candlesticks that referenced this pull request Nov 30, 2023
This is needed so that we can rely on the metadata in adafruit/circuitpython-build-tools#101

Please make a new tagged release after incorporating this change.

If you don't think you'll be able to deal with this in a timely fashion, please let me know.
jepler added a commit to jepler/CircuitPython_color_picker that referenced this pull request Nov 30, 2023
This is needed so that we can rely on the metadata in adafruit/circuitpython-build-tools#101

Please make a new tagged release after incorporating this change.

If you don't think you'll be able to deal with this in a timely fashion, please let me know.
jepler added a commit to jepler/CircuitPython_equalizer that referenced this pull request Nov 30, 2023
This is needed so that we can rely on the metadata in adafruit/circuitpython-build-tools#101

Please make a new tagged release after incorporating this change.

If you don't think you'll be able to deal with this in a timely fashion, please let me know.
jepler added a commit to jepler/CircuitPython_RGB_SpectrumTools that referenced this pull request Nov 30, 2023
This is needed so that we can rely on the metadata in adafruit/circuitpython-build-tools#101

Please make a new tagged release after incorporating this change.

If you don't think you'll be able to deal with this in a timely fashion, please let me know.
jepler added a commit to jepler/CircuitPython_scales that referenced this pull request Nov 30, 2023
This is needed so that we can rely on the metadata in adafruit/circuitpython-build-tools#101

Please make a new tagged release after incorporating this change.

If you don't think you'll be able to deal with this in a timely fashion, please let me know.
jepler added a commit to jepler/CircuitPython_slider that referenced this pull request Nov 30, 2023
This is needed so that we can rely on the metadata in adafruit/circuitpython-build-tools#101

Please make a new tagged release after incorporating this change.

If you don't think you'll be able to deal with this in a timely fashion, please let me know.
jepler added a commit to jepler/CircuitPython_TemperatureTools that referenced this pull request Nov 30, 2023
This is needed so that we can rely on the metadata in adafruit/circuitpython-build-tools#101

Please make a new tagged release after incorporating this change.

If you don't think you'll be able to deal with this in a timely fashion, please let me know.
jepler added a commit to jepler/CircuitPython_UBoxplot that referenced this pull request Nov 30, 2023
This is needed so that we can rely on the metadata in adafruit/circuitpython-build-tools#101

Please make a new tagged release after incorporating this change.

If you don't think you'll be able to deal with this in a timely fashion, please let me know.
@jepler
Copy link
Member Author

jepler commented Nov 30, 2023

The test failure appears to be due to wrong metadata. I think I've filed PRs with every affected library now.

Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused about whether you mean settings.toml or pyproject.toml.

circuitpython_build_tools/build.py Outdated Show resolved Hide resolved
circuitpython_build_tools/build.py Outdated Show resolved Hide resolved
circuitpython_build_tools/build.py Outdated Show resolved Hide resolved
circuitpython_build_tools/build.py Outdated Show resolved Hide resolved
requirements.txt Show resolved Hide resolved
@jepler
Copy link
Member Author

jepler commented Nov 30, 2023

I have settings.toml on the brain because of circuitpython, but it should be pyproject.toml everywhere here.

@dhalbert dhalbert changed the title Get package name from settings.toml, allow arbitrary files in packages Get package name from pyproject.toml, allow arbitrary files in packages Nov 30, 2023
dhalbert
dhalbert previously approved these changes Nov 30, 2023
Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look fine; proof is in the running.

@jepler
Copy link
Member Author

jepler commented Nov 30, 2023

Trying to think of a creative way to let this land without requiring all those libs to be updated. Could be as simple as a blacklist of things that DON'T get the treatment, or disabling it in the communty bundle for now. (allowing the build process to opt out of using the new thing)

@jepler
Copy link
Member Author

jepler commented Dec 2, 2023

I've implemented a simple blacklist (of values that appear in py_modules today and are wrong) so that this can complete CI without all those related issues being closed.

@jepler jepler requested a review from dhalbert December 2, 2023 21:26
Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracking down the outliers.

@dhalbert dhalbert merged commit 988ed98 into main Dec 3, 2023
2 checks passed
@dhalbert dhalbert deleted the include-binary-files-in-package branch December 5, 2023 03:40
@jepler jepler mentioned this pull request Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants